fix kuadrant CR finalizer #538

eguzki · 2024-04-11T15:51:29Z

What

Fix Kuadrant CR cleaning up resources

When Kuadrant CR was deleted, the clean up process failed and the finalizer was never removed. The issue is specifically when the istio config map was not found, error was raised and the finalizer was never removed.

Fixes #363

The whole auth provider registration in istio/maistra code deserves a refactor. More 💣 may arise

Verification Steps

Setup the environment:

make local-setup

Request an instance of Kuadrant:

kubectl -n kuadrant-system apply -f - <<EOF
apiVersion: kuadrant.io/v1beta1
kind: Kuadrant
metadata:
  name: kuadrant
spec: {}
EOF

Delete Istio

kubectl delete namespace istio-system

kubectl delete namespace istio-operator

Delete Kuadrant CR

kubectl delete kuadrant kuadrant -n kuadrant-system

The delete command should not be stuck and finish successfully.

codecov · 2024-04-11T15:54:49Z

Codecov Report

Merging #538 (23537c8) into main (ece13e8) will increase coverage by 0.73%.
Report is 32 commits behind head on main.
The diff coverage is 40.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #538      +/-   ##
==========================================
+ Coverage   80.20%   80.94%   +0.73%     
==========================================
  Files          64       71       +7     
  Lines        4492     4964     +472     
==========================================
+ Hits         3603     4018     +415     
- Misses        600      639      +39     
- Partials      289      307      +18

Flag	Coverage Δ
integration	`71.89% <40.00%> (+0.61%)`	⬆️
unit	`33.57% <0.00%> (+3.54%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
api/v1beta1 (u)	`71.42% <ø> (ø)`
api/v1beta2 (u)	`88.81% <100.00%> (-2.62%)`	⬇️
pkg/common (u)	`83.01% <ø> (-5.81%)`	⬇️
pkg/istio (u)	`75.14% <100.00%> (+1.23%)`	⬆️
pkg/log (u)	`94.73% <ø> (ø)`
pkg/reconcilers (u)	`∅ <ø> (∅)`
pkg/rlptools (u)	`79.84% <ø> (+0.39%)`	⬆️
controllers (i)	`77.27% <81.96%> (+0.47%)`	⬆️

Files	Coverage Δ
controllers/kuadrant_controller.go	`49.81% <40.00%> (-3.76%)`	⬇️

... and 16 files with indirect coverage changes

KevFan

Verified, LGTM 👍

KevFan · 2024-04-11T17:31:56Z

Build catalog is failing due to quay.io/kuadrant/kuadrant-operator-bundle:fix-kuadrant-cr-finalizer is pushed as bundle image but catalog build is expecting quay.io/kuadrant/kuadrant-operator-bundle:fix-kuadrant-CR-finalizer. Probably related to the branch name using uppercase letters. Happy for this to merge even with this workflow failing since it's unrelated

eguzki · 2024-04-11T18:13:22Z

Got it, never use upper case letters in branches

fix kuadrant CR finalizer

23537c8

eguzki requested a review from a team as a code owner April 11, 2024 15:51

KevFan approved these changes Apr 11, 2024

View reviewed changes

eguzki merged commit e3415e1 into main Apr 11, 2024
20 of 22 checks passed

eguzki deleted the fix-kuadrant-CR-finalizer branch April 11, 2024 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix kuadrant CR finalizer #538

fix kuadrant CR finalizer #538

eguzki commented Apr 11, 2024

codecov bot commented Apr 11, 2024 •

edited

Loading

KevFan left a comment

KevFan commented Apr 11, 2024

eguzki commented Apr 11, 2024

fix kuadrant CR finalizer #538

fix kuadrant CR finalizer #538

Conversation

eguzki commented Apr 11, 2024

What

Verification Steps

codecov bot commented Apr 11, 2024 • edited Loading

Codecov Report

KevFan left a comment

Choose a reason for hiding this comment

KevFan commented Apr 11, 2024

eguzki commented Apr 11, 2024

codecov bot commented Apr 11, 2024 •

edited

Loading